fix(v3): Phase 4 cleanup — 6 Codex-flagged bugs (v0.9)#76
Open
mbreiser wants to merge 1 commit into
Open
Conversation
…ler ticks, convert _unknownKeys (v0.9) Six Codex-flagged bugs in landed Phase 4 work: - A1: #sequenceList drag/drop listeners moved to one-time startup wiring. Previously re-attached on every renderSequence(); a single drop after N renders fired N handlers, inserting N duplicate refs and pushing N undo entries. Surfaced within a normal editing session. - A2: onMoveSequenceEntry and onInsertSequenceRefFromLib now walk selection.index across the full displacement, not just the moved entry. Library drag-drop no longer steals selection focus — it shifts the existing selection through the insert (matches the +Add Ref button's intent of selecting the new entry only for explicit button clicks). - A3: Timeline ruler tick positions are computed piecewise by walking the layout array (rulerXForRealTime), so ticks stay aligned with clamped step widths. Flat pxPerSec drifted as soon as any short step clamped to the 60px / 40px minimum. - A4: isConvertibleBlock rejects entries with non-empty _unknownKeys. Block→ref convert was silently dropping forward-compat fields like retry_on_fail, undermining the Tier 1 unknown-passthrough guarantee. The user-facing error message now enumerates every blocker including forward-compat keys. - A5: _buildSequenceEntry mirrors the parser's positive-integer repetitions validation. Closed a doc/JS-mirror divergence path that D4/paste-import would hit (entry.repetitions = 0 → YAML '0' but mirror = 1 via `|| 1` default). - A6: onMoveCommand defensively early-returns on no-op / out-of-bounds, matching the onMoveSequenceEntry pattern. Avoids pushUndo pollution. Tests: 375/375 (was 369). Added 6 new builder-side validation cases to Suite 10b for A5. Verified in browser: - A1: 30 forced re-renders + 1 simulated library drop adds exactly 1 entry. - A4: Right-click on future_keys block shows full error listing all blockers including `forward-compat keys: retry_on_fail, abort_if`. - A3: Multi_block fixture (mixed durations exercising 60px clamping) shows "1m" tick at x=2447 of a 3516-px ruler — piecewise positioning. Footer bumped to v0.9 | 2026-05-27 19:54 ET. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Six Codex-flagged bugs in landed Phase 4 work, all isolated to already-merged code, all additive in spirit. Per the Round 2 handoff §1 / §3 Tier 1.
experiment_designer_v3.html:1809):#sequenceListdrag/drop listeners moved out ofrenderSequence()into one-time startup wiring. Previously re-attached every render → after N renders a single drop fired N handlers → N duplicate refs + N undo entries.experiment_designer_v3.html:1476):onMoveSequenceEntryandonInsertSequenceRefFromLibwalkselection.indexacross the full displacement, not just the moved entry. Library drag-drop no longer steals selection focus.experiment_designer_v3.html:3187): Timeline ruler tick positions computed piecewise via newrulerXForRealTime()walking the layout array. FlatpxPerSecdrifted once any short step clamped to the 60/40px minimum.experiment_designer_v3.html:1544,js/protocol-yaml-v3.js:1154):isConvertibleBlockrejects entries with non-empty_unknownKeys. Block→ref convert was silently dropping forward-compat fields, undermining the Tier 1 unknown-passthrough guarantee.js/protocol-yaml-v3.js:1022):_buildSequenceEntrymirrors the parser's positive-integerrepetitionsvalidation. Closed a doc/JS-mirror divergence path that D4/paste-import would hit.experiment_designer_v3.html:2611):onMoveCommanddefensively early-returns on no-op / out-of-bounds.Tests: 375/375 (was 369). Added 6 builder-side validation tests to Suite 10b.
Footer bumped to v0.9 | 2026-05-27 19:54 ET.
Test plan
npm test— arena 10/10, v2 137/137, v3 228/228 (375 total)v3_future_keys.yamlblock shows error listingforward-compat keys: retry_on_fail, abort_ifv3_multi_block.yaml(mixed durations exercising 60px clamping) renders "1m" tick at x=2447 of 3516-px ruler — piecewise positioning worksNotes
This unblocks Phase 5 (Variables UX), which lands as a follow-up PR.
🤖 Generated with Claude Code